fix(runtime): composition loading stuck indefinitely on multi-slide decks#694
Conversation
Compositions with external sub-compositions (like apple-presentation with 7 slides) load child compositions via fetch(). The root GSAP timeline is only bound after all external compositions finish loading, but the TransportClock duration was only set during initial setup. When bindRootTimelineIfAvailable runs after the external compositions load, it captures the root timeline but never updates the clock. player.getDuration() continues returning 0, so the player's probe interval never fires the 'ready' event, and the Studio shows "Loading composition" indefinitely. Now bindRootTimelineIfAvailable updates clock.setDuration when the root timeline is late-bound. Guarded with try/catch for the early call site where clock is not yet initialized (temporal dead zone). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
vanceingalls
left a comment
vanceingalls
left a comment
There was a problem hiding this comment.
Post-merge advisory review (this PR is already merged into next). Findings flagged for follow-up, not gating.
Verdict — Approve, 2 follow-ups
Surgical 9-LOC fix with a textbook root cause analysis: state.capturedTimeline is null at initial setup, the clock duration never gets updated when the root timeline late-binds after loadExternalCompositions resolves, the probe interval never fires ready, and the loading overlay hangs. The fix is the minimal correct change.
Follow-ups
-
Test added:
it("plays without captured root timeline when audio has failed")— but no test for the specific late-bind path this PR fixes. I'd want a test that asserts: "whenbindRootTimelineIfAvailableruns afterstate.capturedTimelineis null at initial setup, the clock duration gets updated andgetDuration()returns >0." The current test coverage in PR #694's referencedinit.test.tsdoesn't explicitly cover this path. Reason: the bug was hard to find precisely because there was no test for the late-bind interaction; adding one now codifies the invariant. -
The try/catch around
clock.setDuration(boundDuration)swallows a real error case. Comment says "clock not yet initialized — duration will be set during TransportClock setup" — that's the intended swallow, but ifsetDurationthrows for a different reason (e.g. invalid duration value, internal clock state corruption), it gets silently lost. Reason: hard to debug later. Suggest narrowing the catch: only swallow if it's the specific TDZ error you expect, else re-throw or log.
Important
getSafeTimelineDurationSeconds(state.capturedTimeline, 0)is called twice in the touched function — once existing, once in the new code block. Cache the result.
Nits
- The
if (boundDuration > 0)guard is repeated from the original code path — would be cleaner as a single helper that sets duration and pauses the timeline.
Praise
- The root cause writeup in the PR body is excellent. Line-numbered references to the failing logic, exact explanation of why
state.capturedTimelineis null at line 1978 but bound later, and the precise consequence (player.getDuration() === 0→ probe interval never firesready). This is the gold standard for runtime bug PRs. - Minimal scope: 1 file, 9 LOC. Right size for a critical-path runtime fix.
— Vai
2 participants
Problem
The apple-presentation (and any composition with external sub-compositions loaded via
data-composition-src) shows "Loading composition — Preparing the Studio preview" indefinitely in the Studio.Root cause
Multi-slide compositions load child compositions via
fetch()inloadExternalCompositions. The root GSAP timeline is only available after all children finish loading. But theTransportClockduration was only set once during initial runtime setup (line 1978):At this point,
state.capturedTimelineis null (external compositions haven't loaded yet), soclock.getDuration()stays at 0.When
loadExternalCompositionsfinally resolves andbindRootTimelineIfAvailable()captures the root timeline, it setsstate.capturedTimelinebut never updates the clock.player.getDuration()continues returning 0, so the player's probe interval never fires thereadyevent (which requiresadapter.getDuration() > 0), and the loading overlay stays forever.Fix
bindRootTimelineIfAvailablenow callsclock.setDuration(boundDuration)when it late-binds the root timeline. Guarded with try/catch for the early call site (line 1403) whereclockis in the temporal dead zone.Test plan
🤖 Generated with Claude Code